-
Notifications
You must be signed in to change notification settings - Fork 1k
Support profiling the startup phase of a Worker #8026
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: fe24f1c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
50f8524 to
682fc9b
Compare
|
A wrangler prerelease is available for testing. You can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-wrangler-8026You can reference the automatically updated head of this PR with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/8026/npm-package-wrangler-8026Or you can use npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-wrangler-8026 dev path/to/script.jsAdditional artifacts:cloudflare-workers-bindings-extension: wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-cloudflare-workers-bindings-extension-8026 -O ./cloudflare-workers-bindings-extension.0.0.0-veecac04cc.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-veecac04cc.vsixcreate-cloudflare: npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-create-cloudflare-8026 --no-auto-update@cloudflare/kv-asset-handler: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-cloudflare-kv-asset-handler-8026miniflare: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-miniflare-8026@cloudflare/pages-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-cloudflare-pages-shared-8026@cloudflare/unenv-preset: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-cloudflare-unenv-preset-8026@cloudflare/vite-plugin: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-cloudflare-vite-plugin-8026@cloudflare/vitest-pool-workers: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-cloudflare-vitest-pool-workers-8026@cloudflare/workers-editor-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-cloudflare-workers-editor-shared-8026@cloudflare/workers-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-cloudflare-workers-shared-8026@cloudflare/workflows-shared: npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13309873717/npm-package-cloudflare-workflows-shared-8026Note that these links will no longer work once the GitHub Actions artifact expires.
Please ensure constraints are pinned, and |
71e85b3 to
07d1f70
Compare
d6c31a2 to
650bd73
Compare
650bd73 to
6d62eeb
Compare
emily-shen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pages build already has an outfile, is this supposed to reuse that...? Can pages deploy fail due to startup time, and should this generate a cpuprofile if that's the case?
|
|
||
| const mimeTypeModuleType = flipObject(moduleTypeMimeType); | ||
|
|
||
| export const checkNamespace = createNamespace({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not blocking, especially since its hidden, but it feels odd this has its own command. Do we anticipate other 'check' commands?
...maybe we should make wrangler deploy --dry-run a check command?? (come to think of it, maybe we should resurrect wrangler build for wrangler deploy --dry-run 🤔)
(these are only semi-serious suggestions)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree–we should definitely resurrect wrangler build!
In terms of the command structure here, I went with check startup since it seemed feasible that we might want to have other check * commands in future (check types? check config?), but since this is an alpha command we can always change that later.
Co-authored-by: emily-shen <[email protected]>
It's meant to be doing that, yeah—does that seem buggy to you? |
Ah no that’s fine, the lack of changes in the pages deploy path just threw me for a moment but it makes sense now. Will the cpuprofile be generated automatically if a pages deploy fails? |
It will now! |
irvinebroque
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏🚀
This PR adds two features (separate changesets):
--outfileargument forwrangler deploy, which outputs the exact Worker bundle that Wrangler will upload to the Cloudflare API. This has more information than just--outdirbecause it bakes in bindings & module types.wrangler check startup, which profiles the startup time of a Worker and generates a.cpuprofilefile that can be opened in Chrome DevTools/directly in VSCode for further investigation. Additionally, this is automatically generated when Wrangler encounters a startup time error from the API.wrangler check startupdocs cloudflare-docs#19877